Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Type hinting and code refactoring #6

Merged
merged 32 commits into from
Oct 4, 2023
Merged

Type hinting and code refactoring #6

merged 32 commits into from
Oct 4, 2023

Conversation

Matthieu-LAURENT39
Copy link
Contributor

This PR aims to fix a lot of type-hinting issues, as well as to refactor some code to make it more efficient / easier to maintain.
I'm marking it as a draft for now, as i want to fix some of the more complex typing issues as well.

@Matthieu-LAURENT39
Be more consistant with U as shorthand for Union
a76f7cb
@Matthieu-LAURENT39
Add function overload for _to_path
73c7ac3
@Matthieu-LAURENT39
Fix type hinting in attachments.py
501f44b
@Matthieu-LAURENT39
Proper type hinting for run_blocking
a014534
@Matthieu-LAURENT39
Improve typing in client.py
8325349
@Matthieu-LAURENT39
More typing improvements for client.py
9b17b86
@Matthieu-LAURENT39
simplify code in client.py
08db0c5
@Matthieu-LAURENT39
improve typing in commands.py
f7e5e01
@Matthieu-LAURENT39
Some code improvement in commands.py
5357aab
@Matthieu-LAURENT39
Typing improvements in parsers.py
6095c8d
@Matthieu-LAURENT39
Code improvement in parsers.py
aace357
@Matthieu-LAURENT39
Improve type-hinting in exceptions.py
e241f6d
@Matthieu-LAURENT39
Fix type hinting in check.py
394416c
@Matthieu-LAURENT39
Improve type-hinting in help_command.py
6a37f38
@Matthieu-LAURENT39
Code improvement in string_view.py
75e83d9
@Matthieu-LAURENT39
Fix type-hinting in context.py
24765a5
@Matthieu-LAURENT39
Fixi BaseAttachment import
63bfebc
@Matthieu-LAURENT39
Code refactoring in __main__.py
f427a21
@Matthieu-LAURENT39
Fix typo
bca8449
@Matthieu-LAURENT39
Use casefold for case-insentive string comparing
bbeb6fd
@Matthieu-LAURENT39
Ignore type issue for importing __version__
64a6467
@Matthieu-LAURENT39
some more fixes to client.py
abed218
@Matthieu-LAURENT39
Edit readme about case insensitivity
ab74d6b
@Matthieu-LAURENT39
Merge remote-tracking branch 'upstream/master'
f2bc1c7
@Matthieu-LAURENT39
Fix typing in context.py
8f1c68a
@Matthieu-LAURENT39
Clean up typing in client.py
9236a84
@Matthieu-LAURENT39
Document and refactor client._get_id
606bf49
@Matthieu-LAURENT39
Remove deprecated typing containers (List / Dict)
fe7d9d8
@Matthieu-LAURENT39
Ignore some errors that make sense
0717a64
@Matthieu-LAURENT39
Handle str rooms in edit_message
dd73de6
@Matthieu-LAURENT39
Copy link
Contributor Author

Opening this up for merging, the few remaining type errors require important refactoring and so are out of the scope of this PR

@Matthieu-LAURENT39 Matthieu-LAURENT39 marked this pull request as ready for review October 4, 2023 12:49
@nexy7574 nexy7574 self-assigned this Oct 4, 2023
@nexy7574 nexy7574 added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 4, 2023
nexy7574
nexy7574 requested changes Oct 4, 2023
View reviewed changes
Copy link
Owner

@nexy7574 nexy7574 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple things I think need changing, but other than that lgtm

src/niobot/client.py Outdated Show resolved Hide resolved
src/niobot/utils/parsers.py Show resolved Hide resolved
@nexy7574
Copy link
Owner

nexy7574 commented Oct 4, 2023

LGTM

@nexy7574 nexy7574 merged commit 418348f into nexy7574:master Oct 4, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nexy7574 nexy7574 nexy7574 approved these changes

Assignees

@nexy7574 nexy7574

Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Matthieu-LAURENT39 @nexy7574